Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Due to the nature of this PR, I will keep it updated via rebase instead of merge commits so that the changes are as clear as possible. |
|
oh mamma mia 😆 |
|
Your k8s plugin inspired me 😄 |
|
I feared that 😆 |
|
Jokes aside, it might be good to do the actual code changes in a separate PR so that the nf-runtime part is as clean as possible The nf-runtime refactor was just the easiest way to reveal the circular dependencies |
|
I'll review post 25.04 |
|
Refactored so that the CLI code is in a new module Note the following plugins depend explicitly on the CLI code because they add CLI commands:
Might be possible to remove this dependency by moving some interfaces into the core runtime, but not a big deal either way |
ac8c183 to
bf822ef
Compare
b4b321e to
069653d
Compare
5cc1731 to
fd1408d
Compare
|
I refactored the Couldn't quite do this for |
modules/nextflow/build.gradle
Outdated
| api ('org.yaml:snakeyaml:2.2') | ||
| api ('org.jsoup:jsoup:1.15.4') | ||
| api 'org.iq80.leveldb:leveldb:0.12' | ||
| api 'org.eclipse.jgit:org.eclipse.jgit:7.1.0.202411261347-r' |
There was a problem hiding this comment.
Was this downgraded on purpose? before it was 7.1.1.202505221757
There was a problem hiding this comment.
this is likely an incorrect merge commit, none of the versions should be changed. I will fix it
|
Did I break anything? I see some tests failing. Also trying locally I get |
Actually that's my fault, looks like a regression from #6257 . I will submit a separate fix |
498fd1e to
0faaf13
Compare
|
Found some possible solutions for decoupling nf-k8s, nf-tower, and nf-wave from CLI v1. See top comment for details. |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
43053fb to
6e35936
Compare
|
Now the problem is that I guess we'll have to move the |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
d9fa5cd to
d752bc2
Compare
This PR splits the nextflow module into two modules:
nf-cli-v1: contains only the Launcher and CLI classesnextflow: contains everything (i.e. the "runtime")Benefits:
Notes:
I moved the CLI bits of
ConfigBuilderinto a separate classConfigCmdAdapterin order to keep the core config builder in the runtime. This also provides a nice separation of concerns.I moved
K8sDriverLauncherintonf-cli-v1so that thenf-k8sis CLI-agnostic. This means I have to included nf-k8s as a build-time dep for CLI v1, but CLI v2 will be able to loadnf-k8son-demand as a plugin.I moved
PluginExecAwareintonextflowso that plugins can declare CLI commands without depending onnf-cli-v1. I replaced the config loading here with a minimal config, but this might not be enough, so we might want to think about how much of the config logic should be replicated for plugin commands